Skip to content

Conversation

@h-filali
Copy link
Contributor

This commit updates the ECC random secret scalar generation function header.

I checked the implementation for FIPS 186-5 compliance and updated the header comment accordingly.

In case anyone wants to double check: FIPS 186-5

@h-filali h-filali added the CherryPick:earlgrey_1.0.0 This PR should be cherry-picked to earlgrey_1.0.0 label Oct 30, 2025
* d = (d0 + d1) mod n
* ...where n is the curve order.
*
* This implementation follows FIPS 186-4 section B.4.1, where we
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we update this as well?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think so, thanks for flagging this @johannheyszl !

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes definitely, thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please check the PR again @h-filali ? I can't see this change. Only the p384 file is modified currently.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vogelpi you're right. It's added now.

* d = (d0 + d1) mod n
* ...where n is the curve order.
*
* This implementation follows FIPS 186-4 section B.4.1, where we
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* This implementation follows FIPS 186-5 section A.2.1, where we

Copy link
Contributor

@johannheyszl johannheyszl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @h-filali

@h-filali
Copy link
Contributor Author

I rebased and addressed the comments. Thanks for reviewing @johannheyszl !

Copy link
Contributor

@johannheyszl johannheyszl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@h-filali
Copy link
Contributor Author

Thanks @johannheyszl @andrea-caforio. Merging this.

@h-filali h-filali enabled auto-merge October 31, 2025 10:23
This commit updates the ECC random secret scalar generation
function header.

I checked the implementation for FIPS 186-5 compliance and
updated the header comment accordingly.

Signed-off-by: Hakim Filali <[email protected]>
@h-filali h-filali force-pushed the cryptolib-update-ecc-random-scalar-header branch from 086b5b1 to 012d8ae Compare October 31, 2025 10:52
Copy link
Contributor

@vogelpi vogelpi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@h-filali h-filali added this pull request to the merge queue Oct 31, 2025
Merged via the queue into lowRISC:master with commit 2a8cfaa Oct 31, 2025
47 checks passed
@lowrisc-ci
Copy link

lowrisc-ci bot commented Oct 31, 2025

Successfully created backport PR for earlgrey_1.0.0:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CherryPick:earlgrey_1.0.0 This PR should be cherry-picked to earlgrey_1.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants